-
Notifications
You must be signed in to change notification settings - Fork 463
Add Support for Multiple ItemViews in MediaElement #2807
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add Support for Multiple ItemViews in MediaElement #2807
Conversation
- Replace platform-specific directives with runtime checks in MediaElementMultipleWindowsPage.cs - Change LaunchMode to Multiple in MainActivity.cs - Update Info.plist to support multiple scenes - Simplify UIViewController logic in MauiMediaElement.macios.cs - Update methods to handle multi-window applications in MauiMediaElement.macios.cs - Add SceneDelegate classes for iOS and Mac Catalyst
Added a using directive for CommunityToolkit.Maui.Sample.Constants. Removed the preprocessor directive for the secondWindow variable, replacing it with a nullable Window? type to allow for more flexible handling across platforms.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for multiple ItemsView instances and multi-window scenarios on iOS and MacCatalyst for the MediaElement control in .NET MAUI. Key changes include updating the internal view‐controller resolution logic, adding scene‐delegate classes and Info.plist entries for multi‐scene support, and updating the sample app to demonstrate multiple windows.
- Refactored
MauiMediaElement.macios.csto use handlers instead of reflection forItemsViewresolution - Added
SceneDelegateclasses and Info.plist configuration entries for iOS and MacCatalyst multi‐scene support - Updated sample pages and Android
LaunchModeto demonstrate multiple windows
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/CommunityToolkit.Maui.MediaElement/Views/MauiMediaElement.macios.cs | New handler-based logic for finding host view controllers; removed reflection code |
| samples/CommunityToolkit.Maui.Sample/Platforms/iOS/SceneDelegate.cs | Added empty SceneDelegate class to support multiple scenes |
| samples/CommunityToolkit.Maui.Sample/Platforms/iOS/Info.plist | Added UIApplicationSceneManifest for multi-window support |
| samples/CommunityToolkit.Maui.Sample/Platforms/MacCatalyst/SceneDelegate.cs | Added SceneDelegate class for MacCatalyst |
| samples/CommunityToolkit.Maui.Sample/Platforms/MacCatalyst/Info.plist | Added multi-scene manifest entries |
| samples/CommunityToolkit.Maui.Sample/Platforms/Android/MainActivity.cs | Changed LaunchMode to Multiple for Android multi-window |
| samples/CommunityToolkit.Maui.Sample/Pages/Views/MediaElement/MediaElementMultipleWindowsPage.cs | Removed preprocessor guards to always define secondWindow; missing initialization |
Comments suppressed due to low confidence (2)
src/CommunityToolkit.Maui.MediaElement/Views/MauiMediaElement.macios.cs:97
- The method is named with a
Tryprefix but returns void. Consider renaming it toGetItemsViewOnPageor changing it to return a bool to match theTryconvention.
static void TryGetItemsViewOnPage(List<Page> currentPage, out List<ItemsView> itemsView)
samples/CommunityToolkit.Maui.Sample/Pages/Views/MediaElement/MediaElementMultipleWindowsPage.cs:11
- [nitpick] Constant names should use PascalCase by convention. Consider renaming to
BuckBunnyMp4UrlandElephantsDreamMp4Url.
const string buckBunnyMp4Url = "https://commondatastorage.googleapis.com/gtv-videos-bucket/sample/BigBuckBunny.mp4";
| // We may be using Multi-window support, so we need to traverse all Windows to find the current page in a multi-window application. | ||
| // We then check if the VisualElement is an ItemsView (e.g. CarouselView or CollectionView) and add it to the itemsView list/ | ||
|
|
||
| itemsView = []; |
Copilot
AI
Jul 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The out parameter itemsView is initialized but never populated with the discovered items. You should assign the local itemsViewsOnPage list to itemsView (e.g. itemsView = itemsViewsOnPage;).
| viewController = itemsView?.Where(item => item.Handler is not null) | ||
| .Select(item => GetUIViewController(item.Handler)) | ||
| .FirstOrDefault(viewController => viewController is not null); |
Copilot
AI
Jul 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unconditionally overriding viewController here can set it to null if no matching item is found, losing the fallback to the root view controller. Wrap this assignment in a check (e.g. only override if the query returns a non-null controller).
| viewController = itemsView?.Where(item => item.Handler is not null) | |
| .Select(item => GetUIViewController(item.Handler)) | |
| .FirstOrDefault(viewController => viewController is not null); | |
| var potentialViewController = itemsView?.Where(item => item.Handler is not null) | |
| .Select(item => GetUIViewController(item.Handler)) | |
| .FirstOrDefault(viewController => viewController is not null); | |
| if (potentialViewController is not null) | |
| { | |
| viewController = potentialViewController; | |
| } |
| #endif | ||
| const string buckBunnyMp4Url = "https://commondatastorage.googleapis.com/gtv-videos-bucket/sample/BigBuckBunny.mp4"; | ||
| const string elephantsDreamMp4Url = "https://commondatastorage.googleapis.com/gtv-videos-bucket/sample/ElephantsDream.mp4"; | ||
| readonly Window? secondWindow; |
Copilot
AI
Jul 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
secondWindow is declared but never initialized in the constructor, so OnAppearing will always return early. Reintroduce its assignment using your new URL constants.
|
Sorry I just realized this does not actually work! It does not crash. But it does not provide the correct order of collection view items on page. I still have to work that out. I will continue working on this. |
|
@ne0rrmatrix I have an issue when using My issue is this: on MAUI I think code that searcher for iOS media player parent it's written in a way, that if on a |
|
I've just seen this 😅 |
Description of Change
Add Mac/iOS support for multiple Item Views when used in CollectionView or CarouselView when using MediaElement. This also adds support for Multi-Window on iOS.
ItemViewcontrols #2247PR Checklist
approved(bug) orChampioned(feature/proposal)mainat time of PRAdditional information
This will add feature parity when using CollectionView and CarouselView with iOS and MacOS to match current support in Android and Windows when using Media Element. This adds support for multiple items views and supports multiple windows. This PR is focused on MacOS and iOS only. The required support is already available on other platforms.